This repository was archived by the owner on Jan 29, 2020. It is now read-only.
Detach InjectViewModelListener before forwarding. #258
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Prevents duplicate template rendering.
31 tasks
|
Note that the failing Travis tests here do not appear to be directly related to my changes. (It looks like some middleware-related dependency may have released a new version that removes an interface and breaks some tests). |
|
I wonder who was that idiot who reviewed that change and allowed it to pass. In my defence it was a big PR and there were no tests for detaching listeners when I merged it. |
|
Duplicate of #247 |
|
Thanks for sorting it out! :-) |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A long time ago, this commit was introduced to the Forward plugin: zendframework/zendframework@0b8fc7a. This prevents a problem where forwarding causes multiple InjectViewModelListeners to get attached, which in turn causes the view template to get rendered multiple times. This is a waste of cycles, and in some cases causes malfunctions (in situations where rendering a template has side effects).
It appears that somewhere along the line, this functionality was broken and the test intended to cover it was removed. This PR restores the broken functionality and adds a test to help catch future regressions.
Real life use case: my application has a "flash messages" view helper which checks the session for pending messages, displays them, and then clears the queue from the session. This bug prevents flash messages from displaying whenever the application performs a forward action, since the first render clears the queue, then the second render overwrites the first with no flash message data. For me, this is a critical bug.
Please let me know if there are any questions or if I can help to further refine this code. Thank you!